Skip to content

Let threads joining main finish when main ends - #49

Merged
Project516 merged 1 commit into
masterfrom
p516/thread-exit
Nov 5, 2026
Merged

Project516 merged 1 commit into
masterfrom
p516/thread-exit

Conversation

@Project516

Copy link
Copy Markdown
Owner

A thread that joins the main thread no longer deadlocks when main ends. OpenJDK's java/lang/Thread/UncaughtExceptionsTest.java hung on this: its MainThreadAbruptTermination case starts a non-daemon thread that joins main, and main ends with an uncaught exception.

The VM ran the uncaught exception handler correctly, then waited for the other non-daemon threads before it marked main terminated and woke its joiners. The joining thread waited for main, and main waited for it. Main now exits (Thread.exit, terminated state, joiners woken) before the wait, as HotSpot's launcher detaches main before DestroyJavaVM waits for the remaining threads. An uncaught exception on main still exits with status 1.

Tests:

  • test/java/MainThreadExit.java matches HotSpot: a non-daemon thread joins main, and main ends with an uncaught exception.
  • All eight simulator classes in UncaughtExceptionsTest.java match HotSpot's output and exit status; the test passes and goes into passing.txt.
  • pnpm typecheck and pnpm test pass.

@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Bug Fixes
    • The main thread now exits before the runtime waits for other non-daemon threads. This ensures uncaught exceptions from the main thread are handled promptly, while the runtime continues waiting for other non-daemon threads before shutting down.

The JVM now exits the main thread before waiting for other non-daemon threads. A new Java test checks a worker’s join on the main thread after an uncaught exception. The corresponding jtreg test is added to the passing-test list.

Changes: Main thread exit ordering

Layer / File(s) Summary
Exit ordering and join test
src/jvm.ts, test/java/MainThreadExit.java, test/jtreg/passing.txt
mainGen exits the main thread before waiting for other non-daemon threads. The new test starts a worker that joins the main thread and prints whether it is alive after the join. The jtreg passing-test list includes UncaughtExceptionsTest.java.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

  • Project516/java-web#29: Adds thread-body handling for uncaught exceptions and Java thread exit, which relates to the main-thread exit behavior changed here.
  • Project516/java-web#45: Reports that UncaughtExceptionsTest.java hangs because of thread exit handling, which this change also addresses.

Suggested reviewers: project516-review-bot

Merge Risk: Minimal · up to b6bbe

Threads joining main now wake when main ends, which fixes the deadlock described in the PR. No concrete merge-blocking risk remains. One cosmetic edge case remains: during shutdown hooks, main's reported thread state can briefly differ from TERMINATED.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: allowing threads that join the main thread to finish when main terminates.
Description check Passed The description directly explains the deadlock, the main-thread termination change, the related tests, and the reported validation results.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Simplify code

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

LanguageTool

test/jtreg/passing.txt

LanguageTool could not check every text chunk; analysis is incomplete

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

A worker waits while main takes flight,
The thread exits before the wait.
An exception prints its message clear,
The worker checks that main is gone.
Then shutdown follows, neat and done.

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: fe0f0371-6a24-43fc-9d97-ae38f6232222
Commits

Reviewing files that changed from the base of the PR and between cead124 and b6bbee7.

Files selected for processing: 3

  • src/jvm.ts
  • test/java/MainThreadExit.java
  • test/jtreg/passing.txt

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Recent review details

Context from checks skipped due to timeout: 10

  • GitHub Check: jtreg / run (8)
  • GitHub Check: jtreg / run (3)
  • GitHub Check: jtreg / run (4)
  • GitHub Check: jtreg / run (1)
  • GitHub Check: jtreg / run (6)
  • GitHub Check: jtreg / run (2)
  • GitHub Check: jtreg / run (5)
  • GitHub Check: jtreg / run (7)
  • GitHub Check: CodeRabbit
  • GitHub Check: test
Additional context used

Learnings: 1

Common learnings

Learnt from: CR
Repo: Project516/java-web PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-11-05T01:39:41.735Z
Learning: Source excerpt:
# java-web

## Direction

- Prove it with tests: programs whose output must match HotSpot exactly, and
  OpenJDK's own regression tests where they can run.
Learnt from: CR
Repo: Project516/java-web PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-11-05T01:39:41.735Z
Learning: Source excerpt:
# java-web

## Direction

- Compatibility first. Run JDK classes unmodified and implement what HotSpot
  does for them (natives, VM upcalls, init order). Do not patch or replace JDK
  classes. When behavior differs from HotSpot, HotSpot is right.

@project516-review-bot project516-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR fixes a deadlock when a non-daemon thread joins the main thread and main ends with an uncaught exception. The fix moves exitThread(t) before the loop that waits for other non-daemon threads, so main becomes joinable (terminated state, joiners woken) before the VM waits for the remaining threads. This matches HotSpot's behavior where the launcher detaches main before DestroyJavaVM waits for other threads. A new test MainThreadExit.java verifies the scenario, and UncaughtExceptionsTest.java now passes.


review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict approve

@Project516
Project516 merged commit 327705b into master Nov 5, 2026
16 checks passed
@Project516
Project516 deleted the p516/thread-exit branch November 5, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant